fix(core): inline custom CSS into experience HTML head to prevent style flash#8917
fix(core): inline custom CSS into experience HTML head to prevent style flash#8917darcyYe wants to merge 11 commits into
Conversation
COMPARE TO
|
| Name | Diff |
|---|---|
| .changeset/gentle-otters-cheer.md | 📈 +653 Bytes |
| packages/core/src/middleware/koa-experience-ssr.test.ts | 📈 +6.85 KB |
| packages/core/src/middleware/koa-experience-ssr.ts | 📈 +1.99 KB |
| packages/integration-tests/src/tests/experience/server-side-rendering.test.ts | 📈 +2.1 KB |
| packages/integration-tests/src/ui-helpers/trace.ts | 📈 +381 Bytes |
a130e69 to
d275df2
Compare
The preview-mode test navigated through the demo app, whose OIDC redirect to /sign-in dropped the ?preview=true query, so the server never saw preview mode and inlined the saved custom CSS. Hit the experience entry directly with ?preview=true, mirroring the console preview iframe. Also make Trace.cleanup idempotent: it now clears tracePath before unlinking, so a later test that never started a trace no longer unlinks an already-removed file and fails with ENOENT.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR improves the hosted sign-in experience SSR by inlining configured custom CSS into the initial HTML to eliminate a flash of built-in styles, and hardens SSR data embedding against tag/script breakouts.
Changes:
- Inline
customCssinto the served<head>during experience SSR (skipped for?preview=true). - Add safe serialization for SSR data embedded in inline
<script>by escaping HTML-significant characters. - Expand test coverage for CSS inlining and SSR JSON escaping; adjust trace cleanup behavior for repeated cleanup calls.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/integration-tests/src/ui-helpers/trace.ts | Makes trace cleanup idempotent by clearing tracePath before unlinking. |
| packages/integration-tests/src/tests/experience/server-side-rendering.test.ts | Adds integration tests validating custom CSS inlining behavior (including preview mode). |
| packages/core/src/middleware/koa-experience-ssr.ts | Inlines custom CSS during SSR and introduces serializeSsrData to prevent script-tag breakouts. |
| packages/core/src/middleware/koa-experience-ssr.test.ts | Adds unit tests for CSS inlining/escaping and robustly validates SSR JSON injection. |
| .changeset/gentle-otters-cheer.md | Documents the patch fix for the CSS flash and SSR hardening. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Clear the path first so a repeated cleanup (e.g. a later test in the same suite that never | ||
| // started a trace) is a no-op instead of unlinking an already-removed file. | ||
| const { tracePath } = this; | ||
| this.tracePath = undefined; | ||
| await fs.unlink(tracePath); |
There was a problem hiding this comment.
Good point — fixed in 4cc1db3. Switched to fs.rm(tracePath, { force: true }), which makes a missing file a no-op (matching the idempotency intent) while still surfacing real FS errors instead of swallowing them. The path is still cleared first so a repeated cleanup() is a no-op; there is nothing to retry on test teardown, so preserving the path on failure would not buy us anything here.
| it('should inline custom CSS into the served <head> so it applies on the first paint', async () => { | ||
| await updateSignInExperience({ customCss }); | ||
| const experience = new ExpectExperience(await browser.newPage()); | ||
| await experience.navigateTo(demoAppUrl.href); | ||
|
|
||
| const html = await experience.page.content(); | ||
| expect(html).toContain('data-custom-css'); | ||
| expect(html).toContain(customCss); | ||
|
|
||
| await experience.page.close(); | ||
| }); |
There was a problem hiding this comment.
Agreed — fixed in 4cc1db3. Both new custom-CSS tests now run their body in try and await experience.page.close() in finally, so a failed assertion/navigation no longer leaks the page. (The pre-existing tests above also do not close their pages, but that is out of scope for this PR.)
| const serializeSsrData = (data: SsrData): string => | ||
| JSON.stringify(data) | ||
| .replaceAll('<', '\\u003c') | ||
| .replaceAll('>', '\\u003e') | ||
| .replaceAll('&', '\\u0026'); |
There was a problem hiding this comment.
Good catch — done in 4cc1db3. serializeSsrData now also escapes U+2028 (LINE SEPARATOR) and U+2029 (PARAGRAPH SEPARATOR). Since the payload is embedded as a JS expression (Object.freeze(...)) rather than via JSON.parse, these can break parsing in pre-ES2019 engines if left literal. Each replaceAll is annotated with the character it targets, and a unit test asserts both that the literal separators do not survive in the output and that the value still decodes back to the original after JSON.parse.
| const htmlWithCss = | ||
| customCss && ctx.query.preview !== 'true' | ||
| ? ctx.body.replace( | ||
| '</head>', | ||
| `<style data-custom-css>${customCss.replaceAll(/<\/(style)/gi, '<\\/$1')}</style></head>` | ||
| ) | ||
| : ctx.body; |
There was a problem hiding this comment.
Intentionally leaving this as a literal </head> match. The closing tag comes from our own build template (the Vite/SSR HTML), not from any user-controlled input, so its casing/formatting will not change unexpectedly — and if the template ever did change, the integration test (expect(html).toContain('data-custom-css')) would fail immediately and loudly rather than silently regressing. A case-insensitive regex would add the "replace first match only" footgun without protecting against a realistic failure mode here. Happy to revisit if the template ever becomes dynamic.
| // Extract and parse the injected JSON rather than comparing against a bare `JSON.stringify`, which | ||
| // would diverge from `serializeSsrData`'s `<`/`>`/`&` escaping the moment the mock gains such a char. | ||
| const serialized = /Object\.freeze\((?<json>.+)\)/.exec(ctx.body)?.groups?.json; | ||
| expect(JSON.parse(serialized!)).toEqual({ | ||
| signInExperience: { data: mockSignInExperience }, | ||
| phrases: { lng: 'en', data: phrases }, | ||
| }); |
There was a problem hiding this comment.
Fixed in 4cc1db3. Anchored the capture on the trailing ); (/Object\.freeze\((?<json>.+)\);/) so the greedy match stops at the genuine Object.freeze(...) close, and added expect(serialized).toBeTruthy() before the non-null assertion so a regex miss fails with a clear message instead of an opaque JSON.parse(undefined). Applied the same to the other extraction site for consistency.
- Escape U+2028/U+2029 in serializeSsrData so the inline Object.freeze(...)
expression stays parseable in pre-ES2019 engines
- Use fs.rm({ force: true }) in Trace.cleanup so a missing file is a no-op
while real FS errors still surface
- Close pages in finally in the custom-CSS SSR integration tests to avoid
leaking pages on assertion failure
- Anchor the SSR JSON extraction regex on the trailing ); and assert the
match before parsing
Summary
Flash of Unstyled Content (FOUC)
When custom CSS is configured for the sign-in experience, the hosted page briefly rendered the built-in styles before the custom CSS applied (a flash of built-in content). Custom CSS was only injected client-side via
react-helmet, which mutates<head>asynchronously after the page had already painted.The experience SSR middleware now inlines the tenant's
customCssinto a<style data-custom-css>element in the served HTML<head>, so custom CSS is part of the cascade on the first paint.customCssinto<head>before the SSR placeholder substitution, so the</head>match only targets the genuine document head.?preview=true), where the console iframe drives styling live viapostMessage+ react-helmet.</style>sequence in custom CSS so it cannot terminate the<style>element early. The match is on the</styleprefix (not the full closing tag), so every end-tag variant the HTML parser accepts — uppercase, or whitespace before>(</STYLE>,</style >,</style\n>) — is defused.The pre-existing client-side react-helmet
<style>(inAppMeta.tsx) is intentionally kept and does not reintroduce the flash: the server-inlined<style>already styles the first paint, and the helmet tag — added later with identical CSS — produces no visible repaint. It is kept because it is the only custom-CSS path for live preview: the server deliberately skips inlining in preview mode, where the console iframe pushes styling live viapostMessage. On the real page it re-asserts the same CSS (a harmless duplicate that also acts as a precedence backstop). Removing it would break live preview without improving the fix.SSR script breakout hardening
Independent of the FOUC fix, the embedded
window.logtoSsrdata is now serialized through a newserializeSsrDatahelper that escapes</>/&. This is a separate, broader security concern:JSON.stringifyalone is unsafe for embedding inside an inline<script>— any</script>carried in tenant data (custom CSS, custom content, etc.) would close the script element early and enable injection. This affects all experience SSR responses, not only those with custom CSS. The escaped\uXXXXsequences decode back to the original characters when parsed by JS, so values are unchanged afterJSON.parsewhile the HTML parser can no longer recognize any tag delimiters.This affects the production/self-hosted (and Cloud) build path; in development the experience is proxied to Vite and the middleware is a no-op.
Testing
Unit tests, Integration tests
Checklist
.changeset